TOW 967 fix run errors responses#109
Merged
socksy merged 15 commits intofeature/tow-965-make-mcp-server-do-what-it-says-it-doesfrom Sep 29, 2025
Merged
Conversation
Trying to make those tests go green. We had - api errors being printed out to the user rather than the actual result. So we gotta look for those exact errors and return the right thing - wait for both output and status to complete - if status task detects a crash, let's Err(Status::Crashed) ourselves - let runs bubble up errors from following runs (`.await?`) - get rid of now pointless do_follow_run_impl vs do_follow_run split - the side effectful writing is no longer a trouble for the mcp server - end spinner on run successs
- Error messages need to be matched against (fn extract_api_error_message) - Stream run output correctly - needed because those messages are matched by the CLI and integration tests - Return log streams when runs error out, as well as when the succeed - fix a working_dir spot, in tower_deploy (I swear I did this one already) - add cute wee struct for streaming output instead of a tuple
socksy
force-pushed
the
feature/tow-967-fix-run-errors-responses
branch
from
e5669f1 to
c89a1a4
Compare
- Fix the validation error tests' expectation of being deployed or not - check for "nonexistent_param" and return proper error model for undeployed and invalid params errors (cli is not matching http statuses!)
hmm this mock server is getting pretty featureful, when can it replace prod?
The mock server is stateful and with multiple tests running either after another (was getting issues with hello world being deployed when it shouldn't have been), and also with parallel workers, I think it's better we scope each test with their own unique hello_world app name. `test_app` remains non-unique - it needs to be expected to be there (I guess I could put it in the context, but it doesn't need to be yet so 🤷). Other alternative was resetting the mock DB but I figured that might lead to race conditions and it was better if this stuff was just as idempotent as possible
- make sure we can access the app names across steps in a scenario - move to @step everywhere so we can use @when or @then without it complaining - make sure we deploy apps that are supposed to exist in advance - hardcode in the predeployed app, to really communicate that this app is a special case (maybe can be replaced with actual deploying later)
fmt: cargo fmt
…-and-improve-integration-test TOW 966 add CLI regression tests and improve integration test
socksy
merged commit e4a557b
into
feature/tow-965-make-mcp-server-do-what-it-says-it-does
5 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Beware the number! This goes after TOW-965, but before TOW-966 in the stack of PRs!
This is whole bunch of stuff I realized had been fixed in #103 but I somehow missed taking out. And didn't notice until the tests failed to pass. But the upside is I think this PR now has some improvements over #103 even — e.g.
monitor_run_completionused to.unwrap()wait_for_run_completion()... but if that returnsErr()then woe betide the user who got to see a panic error message. Now that's slightly nicer I guess. But literally everything else is the same.So...
do_follow_runintodo_follow_runanddo_follow_run_impl— now unnecessary againmonitor_statusandmonitor_output(like how it is in themainbranch)monitor_statusreturns a status which we can now checksetup_streaming_outputbecause its tuple was getting a bit unwieldy after I was about to add some more stuff to it